Skip to content
This repository has been archived by the owner on Sep 26, 2022. It is now read-only.

feat(NumberField): numeric input component #193

Closed
wants to merge 11 commits into from

Conversation

gristow
Copy link
Contributor

@gristow gristow commented Mar 24, 2021

No description provided.

@vercel
Copy link

vercel bot commented Mar 24, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/thecomputerm/svelte-materialify/BzY1rMCRAsHonRVK2FnT3ucfTodQ
✅ Preview: https://svelte-materia-git-fork-gristow-number-field-component-t-cd3979.vercel.app

@gristow
Copy link
Contributor Author

gristow commented Mar 24, 2021

This component is meant to provide the kind of numeric binding that Svelte provides on <input type="number" /> and preserve all relevant options from TextField.

And so, its styles are entirely shared with TextField. I opted not to create new NumberField.scss and _variable.scss files -- which would have been near duplicates of those for TextField because of this. But, this means quite a few "text-field" classes are applied, and it makes the code structure less clear, since these files don't reside in the same directory as NumberField.

I'd appreciate feedback on the structure: Does it makes more sense to create new .scss files for this component? Should these be extracted to a higher level and shared? If we do that, should we rename "text-field" to something else where it occurs in styles?

Thanks!

@TheComputerM
Copy link
Owner

This will generate duplicate styles, and is this a bug with svelte as we are setting the type to 'number' and it is still returning string?

@gristow
Copy link
Contributor Author

gristow commented Mar 24, 2021

is this a bug with svelte as we are setting the type to 'number' and it is still returning string?

Not a bug with Svelte, per Rich Harris's answer on StackOverflow, the type prop must be static with two-way value binding because the code Svelte generates is different depending on the kind of input.

As a result, even when we set <TextField type="number" /> the value will be a string. See playground example.

This will generate duplicate styles

Yes, I agree -- which is why I wonder about abstracting the styles to a higher level?

Alternately, one sort of super-component could be set to observe the "type" property and swap out html <input /> elements accordingly via an {#if type='number'}...{/if} block?

@TheComputerM
Copy link
Owner

I am thinking of coming up with a new format using svelte:fragments such as:

<InputField>
  <svelte:fragment slot="label">
    Label Text
   </svelte:fragment>
  <input type='text' />
</Input Field>

This would allow us to add textarea, numberic field and text field in one component and not too much extra css.

gristow added 2 commits March 26, 2021 00:41
When `multiple` prop is false, the value of a `<Select />` is a single value. Updated .d.ts to reflect this.
@gristow
Copy link
Contributor Author

gristow commented Mar 26, 2021

In that approach, would <InputField /> replace <TextField />? Or, would <TextField />, <NumberField /> <TextAreaField />, etc..., use <InputField /> as a child component?

(I'd vote for the second option, especially since the accepted props on the different kinds of inputs are different. This would let us define those separately for each input type for the purposes of clear documentation, and also for typescript users.)

I'm not familiar with the <svelte:fragment /> syntax, or I'd offer to do this. (Frankly, I'm only two weeks in to working with Svelte, but I'm loving it.)

In any case, let me know how you want me to proceed with regards to this PR. If you want to create an <InputField /> I'm happy to rewrite <NumberField /> to use it.

@TheComputerM
Copy link
Owner

The only problem with that would be forwarding props. I'll see what I can do to solve this, you can also post here if you have come up with a solution.

@gwelr
Copy link

gwelr commented Apr 9, 2021

A solution here would be great (and we need it as we are engaged in a project where several fields are numeric fields only). For now we are using <input type=number> with custom css applied, but this is far from being optimal. I look forward learning how you plan to solve this. And keep up the good work :-)

@gristow
Copy link
Contributor Author

gristow commented Apr 18, 2021

Since it seems we agree this approach is not the right one, even if we're not decided on a solution, I'm closing and opening issue #224 as a place to discuss a possible solution.

@gristow gristow closed this Apr 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants